fix: features popup for light theme#2196
Conversation
JivusAyrus
requested review from
StarpTech,
thisisnithin and
wilsonrivera
as code owners
WalkthroughUpdated NewFeaturesPopup.tsx to replace hard-coded colors with semantic design tokens and currentColor-driven SVG fills. Updated class names for backgrounds and text, adjusted SVG to inherit color, and kept component logic and exports unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
studio/src/components/dashboard/NewFeaturesPopup.tsx (5)
46-50: Open-in-new: include noopener for safety.Add noopener alongside noreferrer.
- rel="noreferrer" + rel="noopener noreferrer"
56-63: Decorative SVG: mark as presentational and soften color for light theme.Helps screen readers skip it and reduces visual weight on white cards.
- <svg + <svg width="172" height="112" viewBox="0 0 172 112" fill="none" xmlns="http://www.w3.org/2000/svg" - className="text-foreground" + className="text-muted-foreground/40 dark:text-muted-foreground/30" + aria-hidden="true" + focusable="false" + role="presentation" >
55-55: Place SVG behind content to avoid overlaying text.Currently z-40 renders above the card (even if pointer-events-none). Put it below the content’s z-10.
- <div className="pointer-events-none absolute left-4 top-4 z-40"> + <div className="pointer-events-none absolute left-4 top-4 z-0">
192-199: clipPath rect: drop fill attribute.Fill has no effect inside clipPath; removing avoids unintended theming bleed.
<clipPath id="clip0_354_8"> <rect width="171" height="112" - fill="currentColor" transform="translate(0.625)" /> </clipPath>
21-21: Optional: consider tokenizing the gradient stops.Hard-coded HSLA stops won’t adapt to themes; consider design-token-based colors (e.g., from brand/primary scales).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
studio/src/components/dashboard/NewFeaturesPopup.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (1)
studio/src/components/dashboard/NewFeaturesPopup.tsx (1)
22-22: Good move to semantic tokens (bg-card, text-card-foreground, text-muted-foreground).This aligns the popup with themeable design tokens and should fix contrast in light mode.
Also applies to: 25-25, 39-39
2 participants
Summary by CodeRabbit
Checklist